Skip to content

gh-156443: Keep PyLong loop carries as twodigits in shifts and division - #157060

Open
XiaohongGong wants to merge 7 commits into
python:mainfrom
XiaohongGong:gh-156443-longobject
Open

gh-156443: Keep PyLong loop carries as twodigits in shifts and division#157060
XiaohongGong wants to merge 7 commits into
python:mainfrom
XiaohongGong:gh-156443-longobject

Conversation

@XiaohongGong

Copy link
Copy Markdown

gh-156443: Keep PyLong loop carries as twodigits in shifts and division

Several functions in longobject.c (v_lshift, v_rshift, and x_divrem) narrowed a loop carry value to digit or sdigit, then widened it again on the next iteration.

On AArch64, that 64-to-32-to-64 conversion inserts an extra mov on the loop-carried critical path. Keeping the carry at twodigits until the function returns drops that mov and shortens the carry chain. Results are unchanged for valid limbs.

Use v_lshift as an example, the loop on AArch64 previously contains a redundant mov w3, w3 on the carry chain:

  ldr  w0, [x4, x2, lsl #2]    ; a[i]
  mov  w3, w3                  ; carry chain
  lsl  x0, x0, x24             ; a[i] << d
  orr  x0, x0, x3              ; carry chain
  and  w1, w0, #0x3fffffff
  ubfx x3, x0, #30, #32        ; carry chain
  str  w1, [x26, x2, lsl #2]
  add  x2, x2, #1
  cmp  x25, x2
  b.ne

Keeping the carry as twodigits removes that narrowing conversion. The loop is optimized to:

  ldr  w0, [x4, x2, lsl #2]    ; a[i]
  lsl  x0, x0, x24             ; a[i] << d
  orr  x0, x0, x3              ; carry chain
  and  w1, w0, #0x3fffffff
  str  w1, [x26, x2, lsl #2]
  add  x2, x2, #1
  lsr  x3, x0, #30             ; carry chain
  cmp  x25, x2
  b.ne

The instruction count on the loop carried chain is reduced from 3 to 2. We can observe ~20% performance improvement of the
pyperformance pidigits benchmark on an NVIDIA Grace CPU, while no material regressions observed on other platforms and benchmarks.

Tests cover divmod of saturated limbs with quotients near BASE (x_divrem's inner loop), and intra-digit shifts via float() and true division: full-limb values and powers of ten.

Fixes gh-156443.

Co-authored-by: Kyrylo Tkachov ktkachov@nvidia.com

@python-cla-bot

python-cla-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

bedevere-app Bot commented Sep 7, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@XiaohongGong

Copy link
Copy Markdown
Author

Hi, I'm an engineer from NVIDIA that has signed the CLA and PSF. What should I do to pass the CLA check?

@picnixz

picnixz commented Sep 7, 2026

Copy link
Copy Markdown
Member

Hi, I'm an engineer from NVIDIA that has signed the CLA and PSF. What should I do to pass the CLA check?

Be careful with which email you sign the CLA (see https://devguide.python.org/getting-started/pull-request-lifecycle/#why-do-i-need-to-sign-the-cla-again)

@XiaohongGong

Copy link
Copy Markdown
Author

Hi, I'm an engineer from NVIDIA that has signed the CLA and PSF. What should I do to pass the CLA check?

Be careful with which email you sign the CLA (see https://devguide.python.org/getting-started/pull-request-lifecycle/#why-do-i-need-to-sign-the-cla-again)

Thanks for the comment! I confirmed that my company (NVIDIA) has signed the CLA and my email and github are all correct. Any other checks/steps that should I do to pass the CLA?

@StanFromIreland

Copy link
Copy Markdown
Member

You'll have to sign (again) by clicking the button above, I'm afraid otherwise we can't do anything here.

@XiaohongGong

XiaohongGong commented Sep 8, 2026

Copy link
Copy Markdown
Author

You'll have to sign (again) by clicking the button above, I'm afraid otherwise we can't do anything here.

If I sign again by clicking the button above, it means that I will sign on behalf on the individual contributor, which may not be recommended? The contribution is made on behalf of my company. My understanding is that NVIDIA has an existing PSF Contributor Agreement. Could you please let me know whether any additional action is needed to associate this PR/GitHub account with the corporate agreement?

Note that I'v clicked the sign button above and the CLA check has passed. But I'm aware that should be a mistake and I should not do that.

julianknutsen pushed a commit to sjarmak/gascity-packs that referenced this pull request Sep 9, 2026
…presses

Maintainer review fixes for PR gastownhall#217, iteration 1. The two gating defects are
both in coverage_contract.py — the artifact this PR designates the tiebreaker
— and both were reproduced with matched controls before being fixed.

F1 (major) — _strip_code substituted a space for a code span, and a space is
exactly the separator _CLOSING_KEYWORD_REF accepts between a keyword and its
reference. Removing inline code from "Fixes `x` gastownhall#42" therefore *created* the
keyword->reference adjacency GitHub does not honour: the stripper minted
mechanical `closing-keyword` coverage for a body that closes nothing. This is
the expensive direction — a body shaped like "fixes `--resume` #3849" makes
the tiebreaker instruct an agent to bury a live issue. Code is now replaced
by a NUL sentinel, which separates the two sides without ever joining them.

F2 (major) — the fence-close alternative is anchored on a MULTILINE `$` that
a trailing \r sits in front of, so a properly closed CRLF fence read as
unclosed and the `\Z` branch blanked every reference after it. CRLF is the
delivery form the GitHub API actually returns for web-authored bodies, and
"description, code block, Fixes #N at the bottom" is the standard template.
Line endings are now normalised before matching. Confirmed against the real
body review identified: python/cpython#157060 classified no-coverage-evidence
before this change and yields {156443} after. Folded in the same-character
fence-close rule while reworking the regex — a ``` block "closed" by a ~~~
line ended the blanked region early, the same phantom direction as F1.

F3 — mol-pr-triage's verdict branch asserted coverage_contract.py implements
"exactly these rules" while omitting the evidence floor its sibling states and
pins, so a triage agent following that prose could record a Tier-4 demotion
citing a span the shared contract rejects: a false Tier 4, the expensive
direction again, and the one finding all three review lanes filed
independently. The floor is now stated on both sides and mirrored against
_MIN_EVIDENCE_CHARS/_MIN_EVIDENCE_WORDS, and the baseRefName restriction names
$DEFAULT_BRANCH instead of leaving the fetched variable unused.

The suite passed on the shipped tree AND on every fix variant, so green tests
are not evidence here and each new test was verified by breaking the thing it
guards: reverting the sentinel, the CRLF normalisation, the fence
backreference, the floor sentence, the $DEFAULT_BRANCH binding, the repo=
forwarding, and the keyword separator each fail their own assertion. Neither
regression test passes on a stripper that simply deletes everything — the
positive arm is what rules that out.

Picked up while in these files: a gate-level test for
decide_competing_pr_gate(repo=), which 0 of its 13 call sites exercised; a pin
for the no-space `Fixes:gastownhall#42` form, recorded rather than endorsed so a future
relaxation is deliberate; and a fixture comment that paraphrased what it
presented as a verbatim quotation.

tests: 126 passed (was 120). All 6 pack formulas still parse. git diff --check
clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep PyLong loop carries as twodigits in shifts and division

3 participants